deps: float gyp patch for long filenames by addaleax · Pull Request #7963 · nodejs/node

addaleax · 2016-08-03T17:50:51Z

Checklist

make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
commit message follows commit guidelines

Affected core subsystem(s)

tools/gyp

Description of change

Pulling in https://codereview.chromium.org/2019133002/ in its current state, as gyp seems to be largely abandoned as a project.

Original commit message:

Hash intermediate file name to avoid ENAMETOOLONG

Hash the intermediate Makefile target used for multi-output rules
so that it still works when the involved file names are very long.

Since the intermediate file's name is effectively arbitrary, this
does not come with notable behavioural changes.

The `import hashlib` boilerplate is taken directly
from `xcodeproj_file.py`.

Concretely, this makes the V8 inspector build currently fail when long pathnames are involved, notably when using ecryptfs which has a lower file name length limit.

Fixes: #7959
Ref: #7510

Pulling in https://codereview.chromium.org/2019133002/ in its current state, as gyp seems to be largely abandoned as a project. Original commit message: Hash intermediate file name to avoid ENAMETOOLONG Hash the intermediate Makefile target used for multi-output rules so that it still works when the involved file names are very long. Since the intermediate file's name is effectively arbitrary, this does not come with notable behavioural changes. The `import hashlib` boilerplate is taken directly from `xcodeproj_file.py`. Concretely, this makes the V8 inspector build currently fail when long pathnames are involved, notably when using ecryptfs which has a lower file name length limit. Fixes: nodejs#7959 Ref: nodejs#7510

addaleax · 2016-08-03T17:52:19Z

CI: https://ci.nodejs.org/job/node-test-commit/4392/

/cc @bnoordhuis @thefourtheye

thefourtheye · 2016-08-03T18:12:24Z

LGTM

bnoordhuis · 2016-08-03T18:25:58Z

tools/gyp/pylib/gyp/generator/make.py

+
+      # Hash the target name to avoid generating overlong filenames.
+      cmddigest = _new_sha1(command if command else self.target).hexdigest()
+      intermediate = "%s.intermediate" % (cmddigest)


Nitpick but the parens aren't necessary.

bnoordhuis · 2016-08-03T18:26:41Z

LGTM. Just curious, why did you name the variable _new_sha1 instead of e.g. sha1?

addaleax · 2016-08-03T18:31:19Z

That was copied from xcodeproj_file.py, too; I’m changing it, just sha1 sounds good to me too.

thefourtheye · 2016-08-03T18:33:31Z

Actually, leading _ is correct, according to PEP-8. Quoting it,

Even with __all__ set appropriately, internal interfaces (packages, modules, classes, functions, attributes or other names) should still be prefixed with a single leading underscore.

addaleax · 2016-08-03T18:35:55Z

Sounds fine to me, I’ve left it and just dropped the extra parentheses.

jasnell · 2016-08-03T19:11:44Z

LGTM

saghul · 2016-08-04T15:26:31Z

tools/gyp/pylib/gyp/generator/make.py

+# available, avoiding a deprecation warning under 2.6.  Import sha otherwise,
+# preserving 2.4 compatibility.
+try:
+  import hashlib


since GYP is kinda abandoned and you're floating the patch, you can remove all that and just import hashlib. Python 2.4 predates YouTube, keeping compatibility with that is insane these days.

@saghul Done! :)

saghul · 2016-08-04T16:29:30Z

LGTM! 👍

jasnell · 2016-08-05T19:48:29Z

There was red in the previous CI run that looks like a build bot failure. Trying again just to be safe... CI: https://ci.nodejs.org/job/node-test-pull-request/3543/

addaleax · 2016-08-08T18:38:21Z

CI again: https://ci.nodejs.org/job/node-test-commit/4458/

addaleax · 2016-08-08T20:48:54Z

Landed in ~~00dc41d0a033f5ea38953c73cf2fe23f4a8ce577~~ 5111e78 … I can finally build master without flags again! 🎉 😄

Pulling in https://codereview.chromium.org/2019133002/ in its current state, as gyp seems to be largely abandoned as a project. Original commit message: Hash intermediate file name to avoid ENAMETOOLONG Hash the intermediate Makefile target used for multi-output rules so that it still works when the involved file names are very long. Since the intermediate file's name is effectively arbitrary, this does not come with notable behavioural changes. The `import hashlib` boilerplate is taken directly from `xcodeproj_file.py`. Concretely, this makes the V8 inspector build currently fail when long pathnames are involved, notably when using ecryptfs which has a lower file name length limit. Fixes: #7959 Ref: #7510 PR-URL: #7963 Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>

MylesBorins · 2016-09-30T18:13:17Z

@addaleax I've backported this to v4.x

please let me know if it shouldn't have been

bnoordhuis · 2016-10-08T08:28:48Z

@addaleax I just noticed, you never assigned a reviewer in https://codereview.chromium.org/2019133002, that's probably why it's still open. I'd try one of bradnelson@, mark@ or thakis@.

Pulling in https://codereview.chromium.org/2019133002/ in its current state, as gyp seems to be largely abandoned as a project. Original commit message: Hash intermediate file name to avoid ENAMETOOLONG Hash the intermediate Makefile target used for multi-output rules so that it still works when the involved file names are very long. Since the intermediate file's name is effectively arbitrary, this does not come with notable behavioural changes. The `import hashlib` boilerplate is taken directly from `xcodeproj_file.py`. Concretely, this makes the V8 inspector build currently fail when long pathnames are involved, notably when using ecryptfs which has a lower file name length limit. Fixes: nodejs#7959 Ref: nodejs#7510 PR-URL: nodejs#7963 Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>

Pulling in https://codereview.chromium.org/2019133002/ in its current state, as gyp seems to be largely abandoned as a project. Original commit message: Hash intermediate file name to avoid ENAMETOOLONG Hash the intermediate Makefile target used for multi-output rules so that it still works when the involved file names are very long. Since the intermediate file's name is effectively arbitrary, this does not come with notable behavioural changes. The `import hashlib` boilerplate is taken directly from `xcodeproj_file.py`. Concretely, this makes the V8 inspector build currently fail when long pathnames are involved, notably when using ecryptfs which has a lower file name length limit. Fixes: #7959 Ref: #7510 PR-URL: #7963 Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>

Pulling in https://codereview.chromium.org/2019133002/ in its current state, as gyp seems to be largely abandoned as a project. Original commit message: Hash intermediate file name to avoid ENAMETOOLONG Hash the intermediate Makefile target used for multi-output rules so that it still works when the involved file names are very long. Since the intermediate file's name is effectively arbitrary, this does not come with notable behavioural changes. The `import hashlib` boilerplate is taken directly from `xcodeproj_file.py`. Concretely, this makes the V8 inspector build currently fail when long pathnames are involved, notably when using ecryptfs which has a lower file name length limit. Fixes: nodejs/node#7959 Ref: nodejs/node#7510 PR-URL: nodejs/node#7963 Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>

addaleax added build Issues and PRs related to build files or the CI. tools Issues and PRs related to the tools directory. labels Aug 3, 2016

addaleax mentioned this pull request Aug 3, 2016

v8 inspector build failure: File name too long #7959

Closed

addaleax force-pushed the gyp-filename-float branch from 4ab697d to 30a0d62 Compare August 3, 2016 18:07

bnoordhuis reviewed Aug 3, 2016
View reviewed changes

fix nit

b03cca1

saghul reviewed Aug 4, 2016
View reviewed changes

no python 2.4 support

b31e34c

addaleax closed this Aug 8, 2016

addaleax deleted the gyp-filename-float branch August 8, 2016 20:48

cjihrig mentioned this pull request Aug 10, 2016

v6.4.0 Release Proposal #8020

Closed

cjihrig mentioned this pull request Aug 11, 2016

v6.4.0 Proposal #8070

Merged

MylesBorins added the land-on-v4.x label Sep 30, 2016

MylesBorins mentioned this pull request Oct 26, 2016

V4.6.2 proposal #9298

Closed

Uh oh!

Conversation

addaleax commented Aug 3, 2016

Checklist

Affected core subsystem(s)

Description of change

Uh oh!

addaleax commented Aug 3, 2016

Uh oh!

thefourtheye commented Aug 3, 2016

Uh oh!

bnoordhuis Aug 3, 2016

Choose a reason for hiding this comment

Uh oh!

bnoordhuis commented Aug 3, 2016

Uh oh!

addaleax commented Aug 3, 2016

Uh oh!

thefourtheye commented Aug 3, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

addaleax commented Aug 3, 2016

Uh oh!

jasnell commented Aug 3, 2016

Uh oh!

saghul Aug 4, 2016

Choose a reason for hiding this comment

Uh oh!

addaleax Aug 4, 2016

Choose a reason for hiding this comment

Uh oh!

saghul commented Aug 4, 2016

Uh oh!

jasnell commented Aug 5, 2016

Uh oh!

addaleax commented Aug 8, 2016

Uh oh!

addaleax commented Aug 8, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MylesBorins commented Sep 30, 2016

Uh oh!

bnoordhuis commented Oct 8, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

thefourtheye commented Aug 3, 2016 •

edited

Loading

addaleax commented Aug 8, 2016 •

edited

Loading